Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config to enable transform on valueless attrs #241

Merged
merged 5 commits into from
Feb 13, 2020

Conversation

steventsao
Copy link
Contributor

The current codemod skips on transforming nodes with valueless attributes. This prevents a full migration for those who use libraries like ember-test-selectors.

This PR adds the config includeValuelessDataAttributes so the codemod includes data attributes that are valueless.

#237

block
</Common::AccordionComponent>

<XFoo data-foo @name=\\"Sophie\\" />
Copy link
Contributor

@suchitadoshi1987 suchitadoshi1987 Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO This transformation is not correct since the component itself will not see data-foo as a string. It would be whatever value data-foo references to.
data-test is special because of ember-test-selectors, and hence we should only assume this logic for data-test selectors and not for others.
@rwjblue thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a good example is here

{{#link-to
      "cart"
      data-test-foo
      class="nav-actions__cart link--wo-style"
    }}

Will render as:

<a data-test-foo href="/cart" id="ember183" class="nav-actions__cart link--wo-style ember-view">      
</a>

whereas:

{{#link-to
      "cart"
      data-foo
      class="nav-actions__cart link--wo-style"
    }}

would output:

<a href="#" id="ember183" class="nav-actions__cart link--wo-style loading ember-view">      
 </a>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I've updated my PR so it only transforms data-test attributes.

transforms/angle-brackets/transform.js Outdated Show resolved Hide resolved
transforms/angle-brackets/transform.js Outdated Show resolved Hide resolved
Copy link
Contributor

@suchitadoshi1987 suchitadoshi1987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tylerturdenpants tylerturdenpants merged commit 7e06914 into ember-codemods:master Feb 13, 2020
backspace added a commit to hashicorp/nomad that referenced this pull request May 29, 2020
This is the result of running the codemod with this config:
{
  "includeValuelessDataTestAttributes": true
}

I applied the same line-length-preserving manual formatting
as in the previous commit. Thanks to @steventsao for
ember-codemods/ember-angle-brackets-codemod#241
backspace added a commit to backspace/ember-angle-brackets-codemod that referenced this pull request May 29, 2020
backspace added a commit to backspace/ember-angle-brackets-codemod that referenced this pull request May 29, 2020
backspace added a commit to backspace/ember-angle-brackets-codemod that referenced this pull request May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants